docs(web/guides): document CORS allow-list defaults drift on 3.x→4.x migration - #2721
Conversation
…migration Add a "Migrating from global settings to the middleware" subsection to the 3.x→4.x upgrade guide comparing the legacy accessControlAllowHeaders default to the Cors constructor default, calling out the silently-dropped X-Auth-Token, X-Requested-By, and Origin headers, and showing the explicit-constructor-args fix. Add a matching common-issues entry and a migration callout Aside on both the CORS reference page and upgrade guide for both v4-0-0 and v4-0-1-snapshot. Fixes #2708 Signed-off-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot Reviewer A - Full review: The PR documents silent header allow-list shrinkage from 3.x set(accessControlAllow*) to 4.x wheels.middleware.Cors. Two nits: (1) Failure mode description is slightly inaccurate - the server DOES return Access-Control-Allow-Headers on preflights (with the origin in allowlist), it just omits the legacy headers like X-Auth-Token. Developer would see the header in curl -v output. Accurate framing: server returns the header but without the missing headers. The cors.mdx Aside already says silently drops those headers - upgrade guide should match. (2) Methods difference prose under-describes the table - there is also a PATCH/PUT order change (legacy: PATCH, PUT; middleware: PUT, PATCH) that prose does not mention. Suggest: The methods difference (spaces and a minor PATCH/PUT reordering) is cosmetic -- HTTP implementations trim list values and CORS method checks are presence-based. No action is needed unless you pattern-match the exact string. Commits clean. Security: no concerns. Key facts verified against Cors.cfc and security.cfm. Both v4-0-0 and v4-0-1-snapshot in sync. CHANGELOG present.
Wheels Bot — Reviewer B (round 1)A's review is substantively sound. Both findings are accurate, no sycophancy detected. I verified the core claim directly against SycophancyNone detected. A cited specific source files ( False positivesNone detected.
Missed issuesOne minor gap A did not flag: the middleware default adds Verdict alignmentA's COMMENTED state is consistent with two minor nits on a docs-only PR, though finding #1 is concrete enough that request-changes would also have been defensible. ConvergenceAligned, changes needed. The failure mode description in both
to something like:
This matches |
Fix inaccurate failure-mode description in the CORS migration section: the middleware always emits Access-Control-Allow-Headers (Cors.cfc:74 calls cfheader unconditionally) — the response header just omits the dropped values rather than being absent. - web/sites/guides/src/content/docs/v4-0-0/upgrading/3x-to-4x.mdx:114 - web/sites/guides/src/content/docs/v4-0-1-snapshot/upgrading/3x-to-4x.mdx:114 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
Skipped (not in consensus):
The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
This PR adds documentation for the CORS allow-list defaults drift that occurs when migrating from 3.x set(accessControlAllow*) global settings to wheels.middleware.Cors. The round-1 fix (latest commit) corrects the inaccurate failure-mode description flagged in the previous review cycle. Both v4-0-0 and v4-0-1-snapshot are kept in sync. Overall this is clean and accurate; a couple of minor nits remain.
Verdict: comment — no blocking correctness, cross-engine, or security findings.
Correctness
Nit 1 — Methods prose omits the PATCH/PUT ordering change
File: web/sites/guides/src/content/docs/v4-0-0/upgrading/3x-to-4x.mdx, line 128 (same line in the v4-0-1-snapshot copy)
The table shows two differences between the legacy and middleware method defaults, not one:
| Difference | Legacy | Middleware |
|---|---|---|
| Whitespace | GET, POST, PATCH, PUT, … |
GET,POST,PUT,PATCH,… |
| PATCH/PUT order | …PATCH, PUT… |
…PUT,PATCH… |
But the prose only names one: "The methods difference (spaces vs no spaces between list items) is cosmetic." A reader who notices the reordering will be unsure whether they misread the table or the prose is incomplete. Both differences are cosmetic, so the conclusion stands — but the prose should name both:
The methods difference (no spaces after commas, and PATCH/PUT order swapped) is cosmetic —
HTTP implementations trim list values and CORS method checks are presence-based, not
order-sensitive. No action is needed unless you pattern-match the exact string.
This was raised by Reviewer A on the previous SHA and was not addressed by the round-1 fix.
Nit 2 — "no entry in your server logs" could mislead on what is logged
File: web/sites/guides/src/content/docs/v4-0-0/upgrading/3x-to-4x.mdx, line 114 (same line in the v4-0-1-snapshot copy)
The sentence ends: "… and the browser blocks the real request with no entry in your server logs."
This is technically accurate — the real (non-preflight) request is blocked browser-side, so the server never receives it. However, the OPTIONS preflight does appear in HTTP access logs (Cors.cfc line 74 emits Access-Control-Allow-Headers and returns 200 normally). A developer tailing access logs will see the OPTIONS 200 but may not connect it to the client-side CORS failure.
Suggested rewording: "… and the browser blocks the actual request (the POST, PUT, etc.) before sending it, leaving no server-side error to diagnose — only a CORS error in the browser console."
The current text is not wrong; this is a clarity suggestion only.
Docs
Verified facts against source:
- Legacy
allowHeadersdefault:vendor/wheels/events/init/security.cfmline 22 →"Origin, Content-Type, X-Auth-Token, X-Requested-By, X-Requested-With"✓ - Legacy
allowMethodsdefault: same file line 19 →"GET, POST, PATCH, PUT, DELETE, OPTIONS"✓ - Middleware
allowHeadersdefault:vendor/wheels/middleware/Cors.cfcline 22 →"Content-Type,Authorization,X-Requested-With"✓ - Middleware
allowMethodsdefault: same file line 21 →"GET,POST,PUT,PATCH,DELETE,OPTIONS"✓ Asidecomponent imported atcors.mdxline 9 — no missing import ✓- Anchor
#migrating-from-global-settings-to-the-middlewarematches the####heading slug in both doc versions ✓ - CHANGELOG
[Unreleased]entry present ✓
Commits
Both commits conform to commitlint.config.js:
docs(web/guides): document CORS allow-list defaults drift on 3.x→4.x migration✓docs(web/guides): address Reviewer A/B consensus findings (round 1)✓
DCO Signed-off-by trailers present on both commits ✓
Security
No concerns. Pure documentation change; the code example is a recommended middleware constructor call with no injectable values.
Manually triggered baseline refresh via .github/workflows/refresh-visual-baselines.yml on branch docs/bot-2708-wheels-middleware-cors-defaults-differ-in-spelling. Run when an intentional content/layout change makes the visual-regression check fail. The new PNG(s) under web/tests/visual-baselines/ are now the expected rendering; re-run the failing visual-regression job to flip the check green.
1 participant
This PR adds documentation for the silent allow-list shrinkage that occurs when a 3.x app migrates from
set(accessControlAllow*)global settings to thewheels.middleware.Corsconstructor. The middleware'sallowHeadersdefault (Content-Type,Authorization,X-Requested-With) is narrower than the legacy global default (Origin, Content-Type, X-Auth-Token, X-Requested-By, X-Requested-With), causing preflight rejections for any client that sendsX-Auth-TokenorX-Requested-By— with no server-side log entry to diagnose it.Changes
3x-to-4x.mdx) — new#### Migrating from global settings to the middlewaresubsection inside the "CORS — two paths in 4.0" section: side-by-side defaults table, explanation of the failure mode, and an explicit-constructor-args code example. New entry added to the "Common issues" list.cors.mdx) —<Aside type="caution">block after the configuration options table, cross-linking to the upgrade guide section for users who land on the reference page first.v4-0-0andv4-0-1-snapshotdoc versions.CHANGELOG.md[Unreleased]entry added.Fixes #2708
Screenshots needed
None — this is a prose and table documentation change with no visual components.